fix: value-based equality and hashCode for BleCharacteristic - #267
Conversation
BleCharacteristic.operator== compared the `properties` list with `!=`, which is List identity comparison, and hashCode used `properties.hashCode` (also identity based). As a result two characteristics with equal content but distinct list instances were never equal and produced different hash codes, breaking the equality contract and Set/Map lookups. Use `listEquals` for comparison and `Object.hashAll` for hashing, matching the pattern already used in ManufacturerData.
There was a problem hiding this comment.
Code Review
This pull request fixes the equality and hashCode implementation of BleCharacteristic to compare properties by value using listEquals and Object.hashAll, and adds corresponding unit tests. The reviewer suggested using Object.hash instead of the bitwise XOR (^) operator to combine hash codes to prevent potential hash collisions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Addressed the hash-combining review comment in 29e19ff by replacing the XOR combination with |
There was a problem hiding this comment.
Pull request overview
This PR fixes BleCharacteristic’s equality/hash semantics so two instances with identical UUID + properties compare equal and can be reliably used as Set elements / Map keys, and adds a regression test plus a changelog entry.
Changes:
- Update
BleCharacteristic.operator==to comparepropertiesby value (listEquals) and updatehashCodeto hash list contents (Object.hashAll). - Add unit tests validating equality + hashCode behavior.
- Add a changelog entry under
2.1.1.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/ble_characteristic_equality_test.dart | Adds regression tests for value-based equality/hashCode behavior. |
| lib/src/models/ble_service.dart | Implements value-based list equality + content-based hashing for properties. |
| CHANGELOG.md | Documents the fix in the unreleased 2.1.1 section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Cursor <cursoragent@cursor.com>
Problem
BleCharacteristic.operator==compares thepropertieslist with!=, which is List identity comparison in Dart, andhashCodeusesproperties.hashCode, which is also identity-based. As a result, two characteristics with identical content are never equal unless they share the exact same list instance:This breaks the
==/hashCodecontract and makesBleCharacteristicunreliable as aSetelement orMapkey (e.g. deduplicating characteristics across discovery runs).Fix
operator==now comparespropertieswithlistEquals.hashCodenow usesObject.hashAll(properties).This matches the pattern already used by
ManufacturerDatain this package.Testing
test/ble_characteristic_equality_test.dartcovering: equal content ⇒ equal + same hash, differing properties ⇒ unequal, differing uuid ⇒ unequal, and Set/Map key usage.flutter test test/ble_characteristic_equality_test.dartlocally (Flutter 3.44.1 / Dart 3.12.1): 4 tests, all passing.Changelog
Added an entry under the unreleased
2.1.1section inCHANGELOG.md.